Skip to content

fix(lineage): surface the ambiguity warning on every diagram surface (#584) - #588

Merged
padak merged 2 commits into
mainfrom
claude/issue-584-lineage-warning-surfaces
Aug 14, 2026
Merged

fix(lineage): surface the ambiguity warning on every diagram surface (#584)#588
padak merged 2 commits into
mainfrom
claude/issue-584-lineage-warning-surfaces

Conversation

@padak

@padak padak commented Aug 14, 2026

Copy link
Copy Markdown
Member

Closes #584.

What was wrong

#579 added the ambiguity warning for an unqualified table id that resolves to several projects, but only the text surfaces got it. Neither mermaid renderer had a channel to carry a warning, so every diagram caller silently dropped it — a Web UI or --format mermaid/er user still saw one project's answer presented as the whole picture. That is #568 again, on other surfaces.

What changed

Flowchart (render_mermaid) takes an optional warnings list and renders each as a standalone note node, styled as a warning and deliberately left unconnected so it reads as a note rather than a graph participant.

ER (render_er_diagram) got the same channel. erDiagram has no free-standing annotation, so the notes ride as one relationship-less "⚠ note" entity. The existing html.escape(quote=True) neutralises the double quotes that would otherwise terminate the attribute comment.

All call sites now pass result.get("warnings"): CLI --format mermaid/html/er, the lineage server browser, and GET /lineage/mermaid in both views.

_find_node removed#579 replaced it with _find_node_candidates and left it with no callers. Keeping it invites a future caller to bypass candidate resolution, which is the original bug.

The bug only a real browser could show

Rendering both diagrams through mermaid revealed the remedy losing its most important part:

Query a specific one with '--upstream/--downstream :in.c-shared.orders'

<project> was gone. Escaping is correct and holds — mermaid renders the escaped entity back into SVG text as a literal <project>, and the browser then drops it as an unknown tag. It looks right in a terminal, passes every string assertion, and is wrong only where a user actually reads it.

The placeholder is now PROJECT:, with a test asserting warning text stays free of angle brackets. Verified by re-rendering: both views now show PROJECT:in.c-shared.orders in full.

Tests

8 new tests: warning reaches both renderers, no note when there is nothing to warn about, escaping holds in both (<script> cannot survive, quotes cannot break out of the ER attribute comment), multiple warnings are numbered, and the angle-bracket regression guard.

ruff, ty, and the lineage + serve suites (372 passed) are clean.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread src/keboola_agent_cli/commands/lineage.py
padak added 2 commits August 14, 2026 11:35
…oo (#584)

#579 gave `lineage show` and the JSON routes an ambiguity warning, but the
mermaid renderer had no way to carry one: a diagram has no metadata channel,
so all three callers dropped it. A Web UI or --format mermaid user therefore
still got one project's answer looking like the whole picture -- the exact
shape of #568, just on a different surface.

render_mermaid now takes an optional `warnings` list and emits each as a
standalone, deliberately unconnected note node, styled as a warning so it does
not read as part of the dependency graph. All three callers pass what the query
result carries: the CLI's --format mermaid/html, the `lineage server` browser,
and GET /lineage/mermaid. Warnings go through the same label escaping as every
other API-derived string (issue #269 sec-05), covered by a test.

Also removes _find_node, which #579 left behind with no callers. Leaving it
invites a future caller to bypass candidate resolution entirely, which is the
bug #568 reported.
… from it

Two follow-ups from review of the flowchart fix.

render_er_diagram had no warnings channel either, so the ER view dropped the
note on both HTTP surfaces (the CLI's --format er was covered only because the
warning goes to stderr before rendering). erDiagram has no free-standing
annotation, so the notes ride as one relationship-less "note" entity; existing
html.escape(quote=True) neutralises the quotes that would otherwise terminate
the attribute comment.

Rendering the diagrams in a real browser then showed the remedy losing its
most important part: '--upstream/--downstream <project>:id' came out as
'--upstream/--downstream :id'. Escaping is correct and holds -- mermaid
renders the escaped entity back into SVG text as a literal <project>, which
the browser drops as an unknown tag. It looks right in a terminal and passes
every string assertion, and is wrong only where a user reads it. The
placeholder is now PROJECT:, with a test asserting warning text stays free of
angle brackets.
@padak
padak force-pushed the claude/issue-584-lineage-warning-surfaces branch from a8ae482 to 5376ef0 Compare August 14, 2026 09:41
@padak padak changed the title fix(lineage): surface the ambiguity warning on the diagram surfaces too (#584) fix(lineage): surface the ambiguity warning on every diagram surface (#584) Aug 14, 2026
@padak
padak merged commit c36f3d5 into main Aug 14, 2026
4 checks passed
@padak
padak deleted the claude/issue-584-lineage-warning-surfaces branch August 14, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lineage: ambiguity warning missing on the mermaid route and the REPL; _find_node is dead code

1 participant